adding run-make test to autodiff #142444
Open
adding run-make test to autodiff #142444
+1,271
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
the
T-compiler
|
You'll need the PrintTA flag, look in the rustc-dev-guide for the exact spelling. That should at least give you some typetree metadata. You likely also want to first merge support for the more specific flag we discussed where you can pass a specific function name. Look into enzyme to find out the name of the flag and grep for how enzyme handles it internally. We likely want to pass that flag to enzyme not via the command line, since we call libEnzyme, which doesn't accept command line flags. I'd focus for the next days on first getting the flags tested and supported, then you can continue with the tests here. |
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
957d82f to
402f34c
Compare
rustbot
added
the
A-LLVM
This comment has been minimized.
This comment has been minimized.
KMJ-007
commented
Jul 1, 2025
| return LLVMRustResult::Failure; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
this changes are for fixing PrintTAFn flag, which was not working, should i create new PR for this?
There was a problem hiding this comment.
No need for it, just make it it's own commit and that the history is clean.
E.g. end up with a first commit fixing the Wrapper, and a second commit introducing the tests.
There was a problem hiding this comment.
Yeah, I will clean the history after I am done writing tests at the end
This comment has been minimized.
This comment has been minimized.
ZuseZ4
reviewed
Jul 1, 2025
tests/ui/autodiff/f64.stderr
Outdated
| updating analysis of val: %2 = load double, ptr %0, align 8, !dbg !384, !noundef !14 current: {} new {[-1]:Float@double} from %3 = fmul double %2, %2, !dbg !385 Changed=1 legal=1 | ||
| updating analysis of val: %2 = load double, ptr %0, align 8, !dbg !384, !noundef !14 current: {[-1]:Float@double} new {[-1]:Float@double} from %3 = fmul double %2, %2, !dbg !385 Changed=0 legal=1 | ||
| updating analysis of val: %3 = fmul double %2, %2, !dbg !385 current: {[-1]:Float@double} new {[-1]:Float@double} from %3 = fmul double %2, %2, !dbg !385 Changed=0 legal=1 | ||
| updating analysis of val: ptr %0 current: {[-1]:Pointer} new {[-1]:Pointer, [-1,0]:Float@double} from %2 = load double, ptr %0, align 8, !dbg !384, !noundef !14 Changed=1 legal=1 |
There was a problem hiding this comment.
I'd assume that the dbg/noundef numbers like 384, 385 and 14 will change soon enough. Best case you replace all fixed constants like 384 and 385 with different variables, there's probably a good regex for it. I.e. on the first use of 384 you assign a name to it, and on all following usages of 384 you assure that it's the same name (value) showing up. There should be examples in Enzyme and the LLVM lit docs on how to do that.
Otherwise at least make sure that all of these two numbers are replaced with an anonymous placeholder, so if they change tests don't break.
Edit: now that we have a UI test ignore the comment about LLVM's lit test, still check if there's a way to avoid test failures if the numbers change. cc @jieyouxu who might have a suggestion.
There was a problem hiding this comment.
I'm not sure how you would meaningfully normalize these. Maybe instead, use FileCheck inside a run-make test?
Otherwise, this is going to break
There was a problem hiding this comment.
i am thinking of using this regex:
//@ normalize-stderr: "!(dbg|noundef) ![0-9]+" -> "!$1 !N"
//@ normalize-stderr: "%[0-9]+" -> "%X"
//@ normalize-stdout: "!(dbg|noundef) ![0-9]+" -> "!$1 !N"
//@ normalize-stdout: "%[0-9]+" -> "%X"
There was a problem hiding this comment.
That normalization can still be fragile, because can't the instruction ordering also change?
There was a problem hiding this comment.
Yes, any time there are multiple subsequent loads, they're independent enough that, while LLVM usually generates them in a reasonably reliable order, we do change things enough in our codebase that sometimes we change those orderings around. So we would want CHECK-DAG for those lines.
There was a problem hiding this comment.
@workingjubilee @jieyouxu these are currently ui tests since we test stderr/stdout output. Should they be something else? I don't think we can use the LLVM LIT test infrastructure in ui, or do we have a Rust equivalent which makes CHECK-DAG work?
In general, I want to make sure that we see all the type analysis info which enzyme deduces written out, even if it's looking fragile for now. His next PRs will start lowering Rust information to llvm metadata, which should be more reliable if we do it properly. In that case Enzyme will need to deduce less types, and we can narrow down what we're actually testing for (which is the metadata generated by us).
There was a problem hiding this comment.
We have "run arbitrary code to execute the test" tests, called "run-make" because they used to be makefiles, but they actually are Rust executables now: https://github.com/rust-lang/rust/blob/master/tests/run-make/README.md
You can use that to run filecheck:
There was a problem hiding this comment.
Yeah, I think using FileCheck within a run-make test is going to be least fragile, because you can also "don't care" about attributes and whatever and only match on stuff you do care about.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-review
|
Some changes occurred in src/tools/enzyme cc @ZuseZ4 |
This comment has been minimized.
This comment has been minimized.
rustbot
added
has-merge-commits
This comment has been minimized.
This comment has been minimized.
ZuseZ4
reviewed
Jul 1, 2025
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
279a3a3 to
3c049de
Compare
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
3c049de to
8e1cdb3
Compare
rustbot
removed
the
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Karan Janthe <[email protected]>
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
8e1cdb3 to
49de074
Compare
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
49de074 to
49510d0
Compare
|
@ZuseZ4 what should i do about UI test? |
|
I'd follow the suggestions from jieyouxu and jubilee to move to run-make tests, so you can match more precisely on the error messages. There are a lot of lines in this PR, and we don't want to spend too much time maintaining it. |
|
This PR modifies cc @jieyouxu |
|
I have migrated tests to run-make test, can you please review it again |
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
22552d5 to
972fb63
Compare
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Karan Janthe <[email protected]>
rust-cloud-vms
bot
force-pushed
the
autodiff-codegen-test
branch
from
972fb63 to
111b54d
Compare
This comment has been minimized.
This comment has been minimized.
|
CI is failing for UI related tests, but after migration, there is no change in UI tests folder, can someon rerun the CI |
|
You can force PR CI to re-run by closing and reopening the PR. But I think the latest failure is genuine? |
Signed-off-by: Karan Janthe <[email protected]>
This comment has been minimized.
This comment has been minimized.
how do i enable experimental feature in run-make tests? |
Signed-off-by: Karan Janthe <[email protected]>
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @ZuseZ4